Fix NIM selection issue #3482
Merged
Fix NIM selection issue #3482
+81
−73
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
cc @olavtar |
andrewballantyne
force-pushed
the
fix-nim-card-support
branch
6 times, most recently
from
5283004
to
a6836d4
Compare
mturley
reviewed
Nov 15, 2024
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3482 +/- ##
==========================================
- Coverage 85.39% 85.39% -0.01%
==========================================
Files 1352 1352
Lines 30861 30869 +8
Branches 8613 8616 +3
==========================================
+ Hits 26354 26360 +6
- Misses 4507 4509 +2
Continue to review full report in Codecov by Sentry.
|
andrewballantyne
force-pushed
the
fix-nim-card-support
branch
from
a6836d4
to
c7cfd68
Compare
|
Rebased on main to get backend support that recently merged. |
mturley
reviewed
Nov 15, 2024
There was a problem hiding this comment.
Tested a case that failed: On a cluster with KServe and NIM enabled, in Project Details when you select a platform you get the Change button to un-select it on the Models tab but not on the Overview tab. Looking into why
Edit: this is just because the NIM enabled state takes some extra time to load and I wasn't being patient. Disregard.
mturley
approved these changes
Nov 15, 2024
There was a problem hiding this comment.
Tested cases (on dashboard shared cluster kserve-pm):
- With modelmesh and kserve enabled, I see both platforms available and selectable/unselectable in the Overview and Models tabs of a project, and the Model Serving page correctly directs me to the project to select a platform / deploy a model.
- With only modelmesh enabled, the Overview and Models tabs of a project both properly autoselect modelmesh, and the Model Serving page correctly directs me to the project to deploy a model.
- With only kserve enabled, the Overview and Models tabs of a project both properly autoselect kserve, and the Model Serving page correctly allows me to deploy a model.
Tested case (on borrowed cluster ai-dev04.kni.syseng.devcluster.openshift.com):
- With kserve and NIM enabled, I see both platforms available and selectable/unselectable in the Overview and Models tabs of a project, and the Model Serving page correctly allows me to deploy a model.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mturley The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
andrewballantyne
added a commit
to andrewballantyne/odh-dashboard
that referenced
this pull request
Nov 15, 2024
* Fix NIM selection issue * Switch back to using a numerical value
openshift-merge-bot bot
pushed a commit
that referenced
this pull request
Nov 15, 2024
* feat: added ability to deploy more than one NIM model. (#3453) * feat: added ability to deploy more than one NIM model. Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: refactored NIM related logic to nimUtils Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: changes to the error handling Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: changes to the error handling Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: changes .some for .forEach Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: deleting secrets fix Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: deploying the same model pvc fix Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: added a unit test for getNIMResourcesToDelete Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: will add a unit test for getNIMResourcesToDelete later Signed-off-by: Olga Lavtar <olavtar@redhat.com> --------- Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: Modify NIM enablement process (#3455) * Modify NIM enablement process * Clean up code and remove unnecessary manifests * Add logic to check the conditions of the odh-nim-account CR * Added more check for enabled integration apps * If failed to fetch integration app status, should show error on the related tile in the enable application page, should not remove the tile * check integration app status in explore application page * Fix lint issue * Fix lint issue * add annotations to the secret and the account CR * Update backend/src/routes/api/integrations/nim/index.ts Co-authored-by: Andrew Ballantyne <8126518+andrewballantyne@users.noreply.github.com> * Update backend/src/routes/api/components/list.ts Co-authored-by: Andrew Ballantyne <8126518+andrewballantyne@users.noreply.github.com> * clean up * feat: listing all NIM accounts in the namespace, returning the first one. Signed-off-by: Olga Lavtar <olavtar@redhat.com> * Avoid mutating object in useWatchIntegrationComponents * Clean up * feat: added logic for displaying the tile correctly Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: changes for enabling Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: updated mock component with the new properties. Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: addressed PR comments with updates and improvements Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: backend bug fix Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: Missed change from previous commit Signed-off-by: Olga Lavtar <olavtar@redhat.com> * feat: fix for the enabled page and enabled.cy.ts Signed-off-by: Olga Lavtar <olavtar@redhat.com> --------- Signed-off-by: Olga Lavtar <olavtar@redhat.com> Co-authored-by: Andrew Ballantyne <8126518+andrewballantyne@users.noreply.github.com> Co-authored-by: Olga Lavtar <olavtar@redhat.com> * Fix NIM selection issue (#3482) * Fix NIM selection issue * Switch back to using a numerical value --------- Signed-off-by: Olga Lavtar <olavtar@redhat.com> Co-authored-by: olavtar <94576904+olavtar@users.noreply.github.com> Co-authored-by: yu zhao <yzha@redhat.com> Co-authored-by: Olga Lavtar <olavtar@redhat.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(I'll have to create an issue -- don't think we have it yet)
Description
Fixes the issue where if KServe is the only thing installed, NIM doesn't appear as an option and it auto-selects KServe.
How Has This Been Tested?
Not having MM installed then installing the NIM tile & then creating a new project
Test Impact
Test fixes -- no new tests
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main